feat(go): add Go bindings via cgo - #454
Conversation
There was a problem hiding this comment.
All reported issues were addressed across 15 files
Shadow auto-approve: would not auto-approve because issues were found.
Tip: instead of fixing issues one by one fix them all with cubic
Re-trigger cubic
Adds a Go binding to pdf-inspector alongside the existing Node.js and
Python bindings, following the same "one small ABI, mirror the core
crate" pattern as napi/src/lib.rs — but via a hand-written C ABI +
cgo instead of napi's marshaling, since Go has no napi/PyO3 equivalent.
go/ is a new Rust crate (cdylib + staticlib) exposing exactly two
functions plus a matching free: pdfinspector_classify,
pdfinspector_extract_text, pdfinspector_free_string. Each returns an
owned JSON envelope ({"ok":true,"result":...} / {"ok":false,"error":...})
rather than a marshaled struct, since plain C has no object-marshaling
story — this keeps the ABI to three functions with no struct layout to
keep in sync across languages. Every entry point wraps its body in
catch_unwind, mirroring napi's catch_panic, since a Rust panic
unwinding across the FFI boundary is undefined behavior.
go/pdfinspector is the Go package: cgo bindings over that ABI, with
${SRCDIR}-relative CFLAGS/LDFLAGS so `go test` works with no env vars
once the Rust library is built once via `cargo build --release` in go/.
Scope is deliberately v1: Classify and ExtractText only (the two
operations an OCR-routing pipeline needs), not the Node binding's
fuller surface (markdown/process_pdf, region/table extraction, vector
grid detection). Extending go/src/lib.rs with the same envelope
pattern is the natural follow-up if there's interest.
Tested against tests/fixtures (text-based classification + extraction,
encrypted-PDF and invalid-input error handling). cargo fmt/clippy clean.
Co-authored-by: Cursor <cursoragent@cursor.com>
Closes the two solvable gaps from review: no CI coverage, and no way to
consume the binding without a local Rust toolchain.
- ci.yml: new `go` job builds/lints/tests the binding from source on
Linux and macOS on every push/PR, mirroring the existing wasm/build
job conventions.
- publish-go.yml: cross-builds libpdf_inspector_go for
darwin-{arm64,x64}/linux-{x64,arm64} and attaches them to a
`go/vX.Y.Z` GitHub Release whenever go/Cargo.toml's version changes
— the Go-ecosystem equivalent of napi's per-platform npm packages,
since Go modules have no binary-artifact registry to publish into.
- pdfinspector/internal/fetchnative + a `go generate` directive:
downloads and checksum-verifies the matching prebuilt library for
the host platform, no-ops if one is already built, and falls back
to a clear "build from source" message elsewhere. `make native` in
go/ remains the from-source path (README documents both).
cgo's cross-compilation cost and the small amount of duplicated JSON
DTO shape between go/src/lib.rs and pdfinspector.go are inherent to
the hand-written-C-ABI approach rather than fixable here; documented
as such in go/README.md's design notes.
Co-authored-by: Cursor <cursoragent@cursor.com>
Expands the ABI from 2 operations (classify, extract_text) to the full non-OCR document-processing surface the napi and Python bindings expose: process_pdf, detect_pdf, extract_pages_markdown, extract_text_with_positions, extract_structure_elements, extract_text_in_regions, extract_tables_in_regions, detect_vector_grid_in_region, and the extract_tables_with_structure family (markdown / cells / auto-fallback). - go/src/params.rs, go/src/results.rs: request/response JSON shapes, split out of lib.rs now that there are 13 operations instead of 2. Two C argument shapes cover all of them — (data, len) for the two that take no options, (data, len, params_json) for everything else — rather than a bespoke C parameter list per function. - go/pdfinspector/pdfinspector.go: matching idiomatic Go API and types (PdfResult, TextItem, PageMarkdown, PageRegionTexts, VectorGridDetection, TsrTableInput, StructuredCell, ...). - go/pdfinspector/pdfinspector_test.go: coverage for every new function against real fixtures (tagged-PDF structure/mcid, region extraction, TSR table structure), mirroring napi/test.mjs. - go/include/pdf_inspector.h, go/README.md: updated to document the full surface. Deliberately still out of scope: OCR (vision/processPdfWithOcr). Unlike this expansion, OCR support means building with Cargo's `ocr` feature and wiring the same PDFium/ONNX-Runtime-path plumbing ci.yml's ocr-runtime job does for the Rust CLI and Node/Python — a materially larger, separable change rather than more of the same ABI pattern. See go/README.md's Scope section. Co-authored-by: Cursor <cursoragent@cursor.com>
Adds ProcessPdfWithOcr, the last function needed for parity with Node/Python's OCR surface (napi's processPdfWithOcr / Python's process_pdf_with_ocr). go/Cargo.toml now builds against the core crate's `ocr` feature, same as napi/Cargo.toml and pyproject.toml's `python` feature — this does not link a native OCR library at build time (PDFium/ONNX Runtime are loaded dynamically at runtime, same as every other binding), only pulls in a materially larger dependency tree to compile. - go/src/params.rs, go/src/results.rs, go/src/lib.rs: pdfinspector_process_pdf_with_ocr, mirroring napi's OcrOptions / OcrPdfResult field-for-field. - go/pdfinspector/pdfinspector.go: ProcessPdfWithOcr + OcrOptions, OcrMode, PageContentSource, OcrPageResult, OcrPageProvenance, OcrTimings, OcrModelIdentity. Confidence/DPI options are pointers rather than bare floats so an explicit 0.0 is distinguishable from "use the core default" (a bug class flagged during review of a different in-flight Go-bindings PR for this repo). Also broadens test coverage per review feedback: - Password-protected PDF decrypt/reject tests, via ProcessPdfWithOcr(..., Mode: OcrOff) since that remains the only entry point in this package accepting a password (matches Node/Python: their plain process_pdf doesn't take one either). - TestConcurrentCalls: parallel goroutines under `go test -race` (now run in CI) to catch shared-mutable-state races in the Rust side, not just Go-side interleaving. - TestCorpus_*: Classify/ExtractText against every fixture PDF in tests/fixtures/, not just the couple of curated fixtures the per-function tests use. Co-authored-by: Cursor <cursoragent@cursor.com>
There was a problem hiding this comment.
All reported issues were addressed across 11 files (changes from recent commits).
Shadow auto-approve: would not auto-approve because issues were found.
Fix all with cubic | Re-trigger cubic
bd73df1 to
8a034dc
Compare
|
@hemenduroy I have started the AI code review. It will take a few minutes to complete. |
There was a problem hiding this comment.
All reported issues were addressed across 16 files
Shadow auto-approve: would not auto-approve because issues were found.
Tip: instead of fixing issues one by one fix them all with cubic
Re-trigger cubic
2905bfa to
4aa8bed
Compare
|
@hemenduroy I have started the AI code review. It will take a few minutes to complete. |
There was a problem hiding this comment.
All reported issues were addressed across 16 files
Shadow auto-approve: would not auto-approve because issues were found.
Tip: instead of fixing issues one by one fix them all with cubic
Re-trigger cubic
4aa8bed to
9f841e5
Compare
|
@hemenduroy I have started the AI code review. It will take a few minutes to complete. |
There was a problem hiding this comment.
All reported issues were addressed across 16 files
Shadow auto-approve: would not auto-approve because issues were found.
Fix all with cubic | Re-trigger cubic
9f841e5 to
7273464
Compare
|
@hemenduroy I have started the AI code review. It will take a few minutes to complete. |
There was a problem hiding this comment.
All reported issues were addressed across 16 files
Shadow auto-approve: would not auto-approve because issues were found.
You've manually re-run cubic several times on this PR. Each manual re-review checks the full PR again and counts toward your usage quota. To preserve your usage limits, we recommend letting cubic automatically review new commits.
Fix all with cubic | Re-trigger cubic
…bustness) Two P1 correctness bugs, both documentation-vs-implementation mismatches rather than behavioral bugs (verified via new regression tests that the implementations already matched Node/Python's real, tested behavior — only my doc comments were wrong): - ProcessPdf's `pages` doc claimed 0-indexed; the core PdfOptions::pages it forwards to is 1-indexed, matching Python's own documented `process_pdf(path, pages=[1, 3, 5])` and napi's untested-but-identical passthrough. Fixed docs to 1-indexed everywhere (lib.rs, header, pdfinspector.go, params.rs); added a cross-check test against ExtractPagesMarkdown (definitively 0-indexed) proving physical-page alignment, not just self-consistency. - ExtractTextWithPositions's `pages` doc had the same issue — 1-indexed in reality, matching the TextItem.Page field the results carry and napi's own tested behavior. Same doc fix + a test that filtering by pages=[1] returns exactly as many items as unfiltered items report Page==1 (catches an off-by-one a naive "same page number comes back" check wouldn't). Other fixes: - pagesParams.Pages / OcrOptions.PageNumbers: removed `omitempty` — it collapsed a Go caller's explicit empty (non-nil) page selection and a nil one to the same "field omitted" JSON, losing the "process zero pages" case entirely. Added a regression test. - publish-go.yml: workflow-wide `contents: write` narrowed to `read`, scoped `write` to only the release job that actually needs it; fixed the version-change check dying on the very first push that introduces go/Cargo.toml (no parent-commit manifest to diff against). - fetchnative: musl-Linux hosts now refuse the (glibc-only) prebuilt download instead of fetching a library that won't load; alreadyBuilt checks only the host's own library extension instead of either platform's (a stale cross-platform artifact no longer produces a false "already built"); extraction now goes through a temp dir so a mid-extraction failure can't leave a partial file that future runs mistake for a complete one; releaseVersion now reads go/Cargo.toml directly instead of duplicating it as a hand-synced literal. Added tests for the failure-cleanup and version-reading behavior. - ci.yml's ocr-runtime job now also builds the Go native library and runs a real PDFium+ONNX Runtime OCR pass (TestProcessPdfWithOcr_ AutoMode_ScannedFixture, skipped outside that job) — previously the `go` job built with the `ocr` feature but only ever exercised OcrOff/ Auto-on-clean-text-PDF, so a regression in the actual OCR dynamic- loading path would have gone undetected. - Doc-only: go/README.md's "two operations" -> "three" (Classify, ExtractText, DetectPdf all take no options); Makefile's stale `gen.go` reference -> the real path; params.rs's PagesParams doc now spells out the per-function indexing instead of a blanket (and, per the above, partly wrong) "0-indexed" claim; pdf_inspector.h's vector-grid doc now notes its three fields are required, not optional-with-defaults. Co-authored-by: Cursor <cursoragent@cursor.com>
7273464 to
a719336
Compare
|
@hemenduroy I have started the AI code review. It will take a few minutes to complete. |
There was a problem hiding this comment.
All reported issues were addressed across 16 files
Shadow auto-approve: would not auto-approve because issues were found.
You've manually re-run cubic several times on this PR. Each manual re-review checks the full PR again and counts toward your usage quota. To preserve your usage limits, we recommend letting cubic automatically review new commits.
Fix all with cubic | Re-trigger cubic
…leness)
- README: clarify libpdf_inspector_go.{so,a}/.{dylib,a} artifact sets are
platform-specific, not a single build's output.
- fetchnative: track which release a fetched library came from via a
.fetchnative-release.json marker, so a version bump in go/Cargo.toml is
no longer silently ignored by an already-present library (locally-built
libraries with no marker are still always trusted, unchanged).
- fetchnative tests: extract the post-extraction "was the expected
library actually part of this download" check into libraryWasInstalled
so the stale-library regression test exercises the real code path
main() uses instead of a parallel hand-rolled assertion; add matching
positive-case and marker-staleness tests.
Co-authored-by: Cursor <cursoragent@cursor.com>
|
@cubic-dev-ai review |
@hemenduroy I have started the AI code review. It will take a few minutes to complete. |
There was a problem hiding this comment.
1 issue found across 3 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="go/pdfinspector/internal/fetchnative/main.go">
<violation number="1" location="go/pdfinspector/internal/fetchnative/main.go:315">
P2: When a stale `.fetchnative-release.json` marker remains in `go/target/release` but the library there was actually rebuilt locally via `cargo build --release` (`make native`, which never updates or removes the marker), the re-fetch path treats it as an out-of-date fetchnative download and overwrites the local build. That breaks the documented guarantee that fetchnative "never overwrites an existing build" and that a local build "takes priority". The marker records only what fetchnative last downloaded, so it can't tell a locally-built library from a fetched one that's merely stale. Guard the re-fetch so it only fires when the directory was provably populated by fetchnative, or respect an explicit local-build signal (e.g. treat a locally-built library as authoritative regardless of marker).</violation>
</file>
Shadow auto-approve: would not auto-approve because issues were found.
Tip: Review your code locally with the cubic CLI to iterate faster.
Fix all with cubic | Re-trigger cubic
There was a problem hiding this comment.
All reported issues were addressed across 16 files
Shadow auto-approve: would not auto-approve because issues were found.
You've manually re-run cubic several times on this PR. Each manual re-review checks the full PR again and counts toward your usage quota. To preserve your usage limits, we recommend letting cubic automatically review new commits.
Fix all with cubic | Re-trigger cubic
Its pages argument is 0-indexed, but PagesNeedingOCR/PagesWithTables/ PagesWithColumns on the returned PagesExtractionResult are 1-indexed (matching the struct's own field comments) -- the indexing summary grouped the whole function under "0-indexed" without carving out this exception the way it already does for DetectPdf/ProcessPdf and Classify. Co-authored-by: Cursor <cursoragent@cursor.com>
|
@cubic-dev-ai review |
@hemenduroy I have started the AI code review. It will take a few minutes to complete. |
There was a problem hiding this comment.
0 issues found across 1 file (changes from recent commits).
Shadow auto-approve: would require human review. This PR adds cgo-based Go bindings with selective OCR support and a custom binary distribution mechanism. Human review is needed for the architectural choice of cgo over WASM and the operational/security implications of the downloader.
Re-trigger cubic
There was a problem hiding this comment.
No issues found across 16 files
Shadow auto-approve: would require human review. This PR introduces Go bindings via cgo, establishing a new architectural direction and a custom binary distribution mechanism that requires human sign-off.
You've manually re-run cubic several times on this PR. Each manual re-review checks the full PR again and counts toward your usage quota. To preserve your usage limits, we recommend letting cubic automatically review new commits.
Re-trigger cubic
|
Ready for review |
Summary
Adds a Go binding (
go/), matching the document-processing surface of thenapi(Node) and Python bindings — including selective OCR — via a hand-written C ABI + cgo.Classify,ExtractText,ProcessPdf,DetectPdf,ExtractPagesMarkdown,ExtractTextWithPositions,ExtractStructureElements,ExtractTextInRegions,ExtractTablesInRegions,DetectVectorGridInRegion,ExtractTablesWithStructure(+Cells/+Auto), andProcessPdfWithOcr. This matches Node's full surface and exceeds Python's (Python doesn't expose the region/table-structure-recovery functions).(data, len)for the two that take no options,(data, len, params_json)for everything else — rather than a bespoke C parameter list per function. Every call returns a JSON envelope ({"ok":true,"result":{...}}/{"ok":false,"error":"..."}), the same patternnapi/src/lib.rsuses (result structs +catch_unwindon every entry point), adapted for a language with no napi/PyO3-equivalent marshaling.go/Cargo.tomlbuilds against the core crate'socrfeature, same asnapi/Cargo.toml/pyproject.toml. PDFium/ONNX Runtime stay dynamically loaded at runtime (not linked at build time) — same as every other binding.go generatedownloads a prebuilt native library from ago/vX.Y.ZGitHub Release for darwin/linux × arm64/x64 (checksum-verified), falling back tocargo build --release(make native) elsewhere..github/workflows/publish-go.ymlcross-builds those release artifacts whenevergo/Cargo.toml's version changes.gojob inci.yml— builds/lints/tests from source on Linux+macOS on every push/PR,go test -race.ProcessPdfWithOcr(..., Mode: OcrOff)), concurrent-caller safety under the race detector, and a corpus smoke test against every fixture intests/fixtures/.Relationship to #292
This overlaps in intent with #292 (
feat(go): add pure Go bindings (CGO=0) via WebAssembly and wazero), which takes a different architecture — compiling towasm32-wasip1and running it with wazero, avoiding cgo entirely. I want to flag that explicitly rather than have this look like an accidental duplicate. The trade-offs as I see them:CGO_ENABLED=0— no C toolchain, no per-platform native binaries, single embeddable.wasmblob. Genuinely valuable for Go teams that avoid cgo on principle.rayon(parallel parsing) for everywasm32target, so a WASM-based binding inherits the same single-threaded ceiling as the existing browser binding — a real throughput cost on large documents that cgo doesn't pay. More significantly, OCR (PDFium + ONNX Runtime, loaded via nativedlopen) is architecturally impossible from inside a WASM sandbox — a wazero-based binding can never grow into OCR support, which is a real limitation givenprocessPdfWithOcrjust shipped for Node/Python (feat(bindings): expose OCR in Node and Python #405) and OCR routing is clearly an active investment area (Document confidence scores and make OCR routing thresholds configurable #266, Expose configurable confidence threshold for OCR routing and document threshold behavior #267). cgo also matches the distribution pattern already proven out fornapi/Python (per-platform native binaries + release matrix) rather than introducing a third paradigm.Happy to have maintainers weigh in on which direction is preferred for the project's Go story — these aren't mutually exclusive if there's appetite for both (e.g. wazero for a "no toolchain, no OCR" tier and cgo for full parity), but I wanted the comparison on the table rather than silently competing.
Test plan
All 63 tests pass; verified against a from-scratch
cargo cleanrebuild. Root-levelcargo fmt --check,cargo clippy -- -D warnings, andcargo testall pass unaffected (no core crate changes).Summary by cubic
Adds cgo-based Go bindings that mirror the Node/Python document-processing API, including selective OCR, giving Go users full parity with the Rust core. Behavior changes: page filters distinguish nil vs empty (an explicit empty slice processes zero pages);
ProcessPdfandExtractTextWithPositionsuse 1-indexed pages;ExtractPagesMarkdownreturns 1-indexed summary fields while its pages argument remains 0-indexed.gocrate with theocrfeature; PDFium and ONNX Runtime load dynamically at runtime. CI builds the Go native library and runs an OCR pass to verify dynamic loading.go/pdfinspectorusesgo generateto fetch prebuilt native libraries viapdfinspector/internal/fetchnative; musl Linux rejects glibc‑only prebuilts and falls back to local builds. Downloads are checksum‑verified, never overwrite existing files, and write a.fetchnative-release.jsonmarker to track the installed release and avoid staleness. README clarifies artifact sets are platform‑specific..github/workflows/publish-go.ymlcross‑builds and uploads per‑platform artifacts togo/pdfinspector/vX.Y.Zreleases with scoped permissions; re‑runs are idempotent.go test -race, a corpus smoke test, and regressions for page indexing, explicit zero‑page selection, and fetchnative staleness; docs now call outExtractPagesMarkdown’s 1‑indexed result fields.Migration
ProcessPdfandExtractTextWithPositions.ExtractPagesMarkdownsummary fields as 1‑indexed.go generate ./...(especially after upgradinggo/pdfinspector) or build once locally; ensure OCR dependencies are available at runtime when using OCR modes.Written for commit fcf3a37. Summary will update on new commits.